fix: unwrap PSObject-wrapped values when serializing Invoke-MgGraphRequest dictionary bodies#3658
Open
Virgil-Bulens wants to merge 2 commits into
Open
Conversation
…quest dictionary bodies Invoke-MgGraphRequest -Body serializes IDictionary bodies with Newtonsoft.Json. Values that came from the PowerShell pipeline (e.g. bare $_ in ForEach-Object) are PSObject wrappers, so Newtonsoft reflected over PowerShell adapted members (such as the Chars indexed property on strings) and failed with a self-referencing loop error before the request was sent. Add a PSObjectJsonConverter that serializes the underlying BaseObject of PSObject-wrapped values at any depth, and projects pure PSCustomObjects into JSON objects, and pass it at the single JsonConvert.SerializeObject call site for dictionary bodies. Also pin the test project's Microsoft.PowerShell.SDK reference to the latest 7.4.x release: 7.5.x targets net9.0 and contributes no assemblies to the net8.0 test build, which silently left the PowerShellStandard.Library stub (whose APIs return null) as the runtime System.Management.Automation, making PSObject-dependent tests impossible. Fixes microsoftgraph#3654
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Invoke-MgGraphRequest -Body JSON serialization failures caused by PowerShell pipeline values being wrapped as PSObject, by ensuring Newtonsoft.Json serializes the underlying CLR BaseObject instead of PowerShell-adapted members that can trigger self-referencing loops.
Changes:
- Added
PSObjectJsonConverterto unwrapPSObjectvalues (and project purePSCustomObjectproperties) during JSON serialization at any nesting depth. - Wired the converter into
InvokeMgGraphRequest.SetRequestContent(HttpRequestMessage, IDictionary)and exposed the method asinternalto enable direct testing. - Added xUnit tests for PSObject/PSCustomObject serialization behavior and pinned
Microsoft.PowerShell.SDKto a net8-compatible 7.4.x version for the test target.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Authentication/Authentication/Helpers/PSObjectJsonConverter.cs | Introduces a Json.NET converter that unwraps PSObject to prevent self-referencing loop serialization failures. |
| src/Authentication/Authentication/Cmdlets/InvokeMgGraphRequest.cs | Uses the new converter when serializing IDictionary request bodies; makes the method internal for test access. |
| src/Authentication/Authentication.Test/Microsoft.Graph.Authentication.Test.csproj | Pins Microsoft.PowerShell.SDK for net8.0 to ensure tests run against a real PowerShell runtime. |
| src/Authentication/Authentication.Test/Helpers/PSObjectJsonConverterTests.cs | Adds coverage for nested PSObject-wrapped values, PSCustomObject projection, and the Invoke-MgGraphRequest wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3654
Changes proposed in this pull request
PSObjectJsonConverter, a NewtonsoftJsonConverterthat unwrapsPSObject-wrapped values to theirBaseObjectduring serialization, at any nesting depth. PurePSCustomObjects (which have no underlying CLR object) are projected into JSON objects from their properties.JsonConvert.SerializeObjectcall site inInvokeMgGraphRequest.SetRequestContent(HttpRequestMessage, IDictionary), so idiomatic hashtable bodies containing pipeline-produced values (bare$_inForEach-Object) serialize the underlying CLR values instead of failing withSelf referencing loop detected for property 'Value' with type 'System.Management.Automation.PSParameterizedProperty'.PSCustomObjectbodies, and theSetRequestContentwiring (madeinternal; the assembly already hasInternalsVisibleTofor the test project).Microsoft.PowerShell.SDKreference to 7.4.17. 7.5.x targets net9.0 only and contributes no assemblies to the net8.0 test build, which silently left thePowerShellStandard.Librarystub (whose APIs return null) as the runtimeSystem.Management.Automation. This is why the existingShouldReturnPsObjecttests fail on net8.0, and it made the new PSObject-dependent tests impossible to run. With the pin, the pre-existingStringUtilTests.ShouldReturnPsObjectfailures on net8.0 are also resolved.Verification
dotnet test src/Authentication/Authentication.Test -f net8.0: 84/85 pass on Linux; the one failure (AuthenticationHelpersTests.ShouldThrowIfNonExistentCertNameIsProvidedAsync) is pre-existing on an unmodified checkout ofmainand unrelated (certificate store).{"message":{"toRecipients":[{"emailAddress":{"address":"recipient@example.com"}}]},"saveToSentItems":true}[pscustomobject]@{ address = "..." }body value, which previously failed the same way, serializes to{"emailAddress":{"address":"recipient@example.com"}}.Other links